Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buckets view feature implementation #567

Merged
merged 28 commits into from
Mar 19, 2024
Merged

Buckets view feature implementation #567

merged 28 commits into from
Mar 19, 2024

Conversation

vkorir
Copy link
Contributor

@vkorir vkorir commented Oct 16, 2023

Details

  • Created Buckets view accessible from route /project/:projectID/board/:boardID/buckets
  • Added guards and included a route /canvas for the canvas view
  • Teachers can enable or disable canvas/workspace/bucket board views on edit/create board
  • Teachers can select what buckets are visible on a board
  • Teachers/Students can move posts between buckets
  • Refactored common menu items into a shared component across canvas/workspace/bucket components

Closes #560

@vkorir vkorir changed the title Vk/bucket view Buckets view feature implementation Oct 16, 2023
Copy link
Contributor

@JoelWiebe JoelWiebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really great work!

  • The Default and Enabled Views options in the board configurations work very well (I appreciated the automated enabling and disabling of enabled views based on defaults)
  • Very clean drag and drop experience

Requests/suggestions:

  • If I set Bucket View as default then use the + button to create new buckets, then I need to refresh the page before I can use the + button to add newly created buckets to the view. Could we have the Bucket UI update once buckets modified to avoid the need for refreshing the page?
  • Could we have a "back" button on the Bucket View (something like we have on the canvas) to take use back to the projects dashboard? P.S. Different styling/icons/text though are fine by me.
Screenshot 2023-10-18 at 10 14 44 AM
  • Could we also add the CK Monitor to the list of views that can be enabled and set as default?
  • What do you think about having icons and labels of all enabled views ("Board", "Buckets", "Workspace", and "Monitor") across the top of the nav bar on the right side (just left of the notifications icon/user's name) as we need a way for users to navigate between views? (The current view could always be disabled). (a) This could be icons and labels when space allows and (b) icons only with tooltips of labels on smaller screen widths.
  • Can we have the list view hidden from all students but remain visible to the teacher (either in the menu or as an icon in the nav bar)?
  • Can we increase the post width so there is only a small space to the left and right sides of the post?
Screenshot 2023-10-18 at 10 41 20 AM
  • The buckets will need to be responsive to large numbers of posts. Can we set the bucket height to grow with the number of posts?
    <img width="386" alt="Screenshot 2023-10-18 at 10 46 33 AM"

  • Since a post can exist in multiple buckets, dragging duplicates of the same post can display visually twice in the same bucket (only for the person who moves the post; resolved by refreshing page). Can we check for duplicate when post is moved and prevent duplicate posts from being displayed)?

Screenshot 2023-10-18 at 12 10 13 PM
  • Did we still want to display posts in two columns within a bucket if there are one or two buckets displayed in the bucket viewer (as we have some extra room)? I'm fine if we move this to a later issue or drop it for now.
Screenshot 2023-10-18 at 12 16 26 PM

Very clean implementation and this really rounds out the platform into a full suite of tools. Thanks for sending this to me for feedback and suggestions! Let me know if any of the above items should be moved to separate issues to be addressed at a later time.

@LunarFang416
Copy link
Contributor

@JoelWiebe Finished implementing the fixes you asked for above. Please let me know if we need to make any more tweaks. Once this is merged in I can work on implementing the other functionality for the bucket view.

Copy link
Contributor

@JoelWiebe JoelWiebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LunarFang416 This is really great work -- thanks for addressing these so quickly.

Suggestions:

  • Could we add horizontal scrolling for the buckets when there is horizontal overflow? I notice there is still a horizontal overflow issue where the outer container doesn't line up with the column containers and the columns can overflow outside of the nav bar. This may also permit us to have more than 4 columns in the future, if we have horizontal scrolling.

Screenshot 2023-11-13 at 4 07 25 PM

  • For the view tabs, can we disable rather than hide the current tab (to help maintain the tab metaphor) then order them as follows: Canvas, Bucket View, Workspace, Monitor?
  • Can we swap the current pencil icon for the canvas with the Dashboard icon: https://fonts.google.com/icons?selected=Material%20Symbols%20Outlined%3Adashboard%3AFILL%400%3Bwght%40400%3BGRAD%400%3Bopsz%4024
  • When loading a large number of posts 10+, for example, the column remains blank for a second or two. I notice that a spinner pops up for a moment but does not remain in place while the posts load. Can we either have that spinner remain in place until all posts are loaded or have a spinner for each column that waits until the containing posts are loaded before disappearing?

Really nice work with multi-column layout within each bucket, reduced padding between posts, and increasing the vertical size of the buckets to allow for large numbers of posts.

@vkorir As you started this work, would you like to review these changes after @LunarFang416 finishes these last few items?

@vkorir
Copy link
Contributor Author

vkorir commented Nov 14, 2023

  • "Add bucket" button takes up a bucket view space unnecessarily, let's put it on the same line as the bucket name but on the far right
  • Let localize the loader to within each bucket so that each bucket will have it's loader, currently it takes the whole view
  • There's some overflow when the bucket width cannot accommodate two posts, check the recording below
Screen.Recording.2023-11-14.at.00.29.28.mov

@JoelWiebe a functional clarification is I'm wondering if we should persist post movements between buckets by students. I'm thinking this could be chaotic when everyone starts moving things around at the same time, maybe we just localize to a browser session?

@LunarFang416
Copy link
Contributor

@JoelWiebe and @vkorir appreciate the feedback! I pushed the changes you requested.

@vkorir
Copy link
Contributor Author

vkorir commented Nov 21, 2023

  • In-bucket loader not working
  • Let move the "+" icon from the view since it takes up space

@JoelWiebe
Copy link
Contributor

JoelWiebe commented Nov 22, 2023

@LunarFang416 Great improvements! @vkorir Thanks for your review and feedback.

  • Localized bucket loaders appear to be working well for me, @vkorir can you help give us steps to reproduce that issue?

  • I've been thinking about whether to persist bucket movement changes or localize within each browser. Our upcoming research study will involve only the teacher moving the posts so this is not an issue, but I would expect future this may involve students moving posts to help sort them for the class or future activities. Let's leave this as-is for now, but would it be feasible (if needed) to port over some of the functionality of locking and real-time updating that we had used in the canvas?

  • To improve readability of disabled view labels, could we update the color, e.g., rgba(255, 255, 255, 0.55)

  • When there are lots of posts the + button for creating a new bucket gets a bit lost (lower in the page); @vkorir what did you have in mind for the + button (should this have an absolute position in the bottom right of the page or be moved elsewhere)?

@LunarFang416
Copy link
Contributor

@JoelWiebe Pushed the small styling changes you requested.

Copy link
Contributor

@JoelWiebe JoelWiebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me!

@vkorir?

@vkorir
Copy link
Contributor Author

vkorir commented Dec 11, 2023

Functionality looks good. Let's do the minor code refactoring and we should be good.

@LunarFang416 LunarFang416 merged commit dfd8f19 into develop Mar 19, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Bucket View
3 participants